Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FlutterFragment predictive back #44865

Closed

Conversation

justinmc
Copy link
Contributor

@justinmc justinmc commented Aug 18, 2023

Following up on #39208 and #42789, which added predictive back support for FlutterActivity.

My test app for this is: https://github.com/justinmc/flutter-add-to-app

I tried this both with the Flutter app as the main activity, and with it as a secondary activity after an initial Android route. Predictive back worked in both cases.

Related: flutter/flutter#109558, and my comment flutter/flutter#109558 (comment).

b/242216228

@justinmc justinmc self-assigned this Aug 18, 2023
@justinmc justinmc force-pushed the flutter-fragment-predictive-back branch 3 times, most recently from 0718e5c to e2ff310 Compare August 18, 2023 22:15
@justinmc justinmc force-pushed the flutter-fragment-predictive-back branch from e2ff310 to 89629fb Compare August 18, 2023 22:16
// can be changed by calling setFrameworkHandlesBack. For example, the
// framework will call this automatically in a typical app when it has
// routes to pop.
onBackPressedCallback.setEnabled(false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a potentially dangerous change. Does the framework guarantee that the callback will always be enabled when applicable via setFrameworkHandlesBack, regardless of API version?

@@ -521,6 +521,7 @@ protected FlutterFragment createFlutterFragment() {
? TransparencyMode.opaque
: TransparencyMode.transparent;
final boolean shouldDelayFirstAndroidViewDraw = renderMode == RenderMode.surface;
final boolean shouldAutomaticallyHandleOnBackPressed = Build.VERSION.SDK_INT >= 33;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this change, you'll need to update the implementation of onBackPressed() below. On Android 33, the onBackPressed() method is still relevant, but it won't behave correctly without a call to super.onBackPressed().

Alternatively, you could make this condition always true and fully remove the onBackPressed() override below. That way you have less conditional behavior.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose removing the onBackPressed() override could impact behavior for any clients that subclass FlutterFragmentActivity and themselves override the method, but given that Android now formally discourages that anyways, that seems like a reasonable breaking change.

@goderbauer
Copy link
Member

I spoke to @justinmc last week and he told me that he will get back to this one soon.

@justinmc
Copy link
Contributor Author

I'll re-evaluate this after #49093 is merged.

@justinmc justinmc closed this Mar 26, 2024
@justinmc
Copy link
Contributor Author

Spiritual successor: #52302

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants